Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always notify Account when FirestoreAccountStorage receives snapshot #49

Merged
merged 3 commits into from
Dec 9, 2024

Conversation

Supereg
Copy link
Member

@Supereg Supereg commented Dec 8, 2024

Always notify Account when FirestoreAccountStorage receives snapshot

♻️ Current situation & Problem

With StanfordSpezi/SpeziAccount#79 we only forward "complete" account details to the setupComplete closure of the AccountSetup view. This requires that the StorageProvider always updates the account details (to clear the incomplete flag) even if there aren't any keys stored in the storage provider. This wasn't the case for FirestoreAccountStorage resulting in the setupComplete closure to never be called and the incomplete flag to never be cleared.

⚙️ Release Notes

  • Fixed an issue where the incomplete flag would never be cleared if there weren't any details stored for an account.

📚 Documentation

--

✅ Testing

--

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@Supereg Supereg requested a review from PSchmiedmayer December 8, 2024 22:24
Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the quick fix @Supereg!

Would be amazing if we can add the small regression test that you noted on Slack; otherwise happy to see it merged 👍

Also looks like one of the tests is currently failing; would be good to investigate this as well: https://github.com/StanfordSpezi/SpeziFirebase/actions/runs/12225668289/job/34099948868?pr=49#step:16:2299

Package.swift Outdated Show resolved Hide resolved
@PSchmiedmayer PSchmiedmayer added the bug Something isn't working label Dec 9, 2024
Copy link

codecov bot commented Dec 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.72%. Comparing base (7c68296) to head (97abf32).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #49      +/-   ##
==========================================
+ Coverage   68.65%   68.72%   +0.08%     
==========================================
  Files          19       19              
  Lines        1199     1202       +3     
==========================================
+ Hits          823      826       +3     
  Misses        376      376              
Files with missing lines Coverage Δ
.../SpeziFirebaseAccount/FirebaseAccountService.swift 73.60% <100.00%> (+0.11%) ⬆️
.../SpeziFirestore/DocumentReference+AsyncAwait.swift 72.55% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c68296...97abf32. Read the comment docs.

@Supereg Supereg merged commit 5dd57f9 into main Dec 9, 2024
8 checks passed
@Supereg Supereg deleted the fix/storage-never-returns branch December 9, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants